account for RangeError: Array buffer allocation failed#303
Conversation
703a7c9 to
d179a97
Compare
|
|
||
| return true; | ||
| } catch { | ||
| return false; |
There was a problem hiding this comment.
I'm not sure how i feel about absolutely swallowing everything here. Should we log the error?
There was a problem hiding this comment.
this only ever fails for one reason, sending on CONNECTING state, imo its fine? connection doesnt have direct access to a logger and i think threading it through just for this doesnt super make sense
we can alternatively trigger this.ws.onerror and close
| import { DeserializeResult, SerializeResult } from '../transport/results'; | ||
| import { coerceErrorString } from '../transport/stringifyError'; | ||
|
|
||
| export class CodecMessageAdapter { |
There was a problem hiding this comment.
we should leave doc strings on all of our classes!
There was a problem hiding this comment.
curious why an adapter instead of making the implementation conform to this
There was a problem hiding this comment.
codec is user substitutable and we cant expect every consumer to remember to try/catch toBuffer fromBuffer, so we do it at this layer
| } | ||
|
|
||
| // transition | ||
| this.pendingSessions.delete(session); |
There was a problem hiding this comment.
what's the significance of moving this up? maybe worth a comment
There was a problem hiding this comment.
this is the point where we 'consume' the pending session and turn it into a connected session
now that there is a fail point, we want to clear the old pending session earlier rather than later
masad-frost
left a comment
There was a problem hiding this comment.
FIVE BIG BOOMS
💥
💥
💥
💥
💥
tiny comments
Why
tldr; a failed allocation can be try-caught by a top-level handler (e.g. in react) and river can pretend nothing went wrong and construct another message
by the time the next message comes around, its possible theres enough memory to allocate again and whoops!!! we skipped sending a message
some more context on this error: https://groups.google.com/a/chromium.org/g/blink-dev/c/pZ9kld0LehA/m/Ho-SCn8tBAAJ
What changed
CodecMessageAdapter) to try catch toBuffer and fromBuffer operationsProtocolError.MessageSendFailureso the consumer can decide to shut down the world in this case tooCLOSINGorCLOSEDstates (safe to discard as we have the sendbuffer)messageFramingcode (we axed UDS a while ago)Versioning